-
Notifications
You must be signed in to change notification settings - Fork 16
Add scalar_fieldmatrix #2289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add scalar_fieldmatrix #2289
Conversation
67fd30e
to
65b036a
Compare
65b036a
to
3f81735
Compare
2405aca
to
02bb025
Compare
d91cf59
to
e1c1c22
Compare
eltype(x.parent) | ||
inner_type_ignore_adjoint(x::DataLayouts.AbstractData) = eltype(x) | ||
inner_type_ignore_adjoint(x::Adjoint) = typeof(x.parent) | ||
inner_type_ignore_adjoint(x) = typeof(x) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this also support when x
is a broadcasted object that contains an Adjoint?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It currently is never used where x
could be broadcasted, but maybe it should be added so this could be used elsewhere? Although this should be used with caution, because it is a bit of a hack (it is only used with axis vectors)
03c0a76
to
ef4ead4
Compare
2c6c054
to
52cf757
Compare
Add a function to convert a FieldMatrix where each matrix entry has an eltype of some struct into a FieldMatrix where each entry has an eltype of a scalar. Add additional tests for scalar_matrixfields Use @test_all in tests Make suggested changes to tests and field_name_dict.jl Revert unrolled_findfirst Clean up field matrix tests and add support for DiagonalMatrixRows CamelCase struct name Clean up tests and get_scalar_keys wip backup Minimal working with allocs WIP1 WIP more allocs fix Assorted cleanup Fix dx/dx case reduce code duplication; fix example Add gpu test further cleanup, extend diagonalrow fix names test and comments Add docs docs bugfix remvoe bad refs fix docs formatting
52cf757
to
13633ca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you Teja! We should eventually try to simplify the scalar indexing logic, as it's ended up very entangled with the AxisTensor internals, but I think you've captured every case we could possibly support. The new section in the docs is also helpful. This will be really useful for implementing the sparse autodiff Jacobian in ClimaAtmos.
closes #2306
This PR adds
scalar_fieldmatrix
, a function which takes in a field matrix, and returns a field matrix with keys that are associated with matrix fields of either uniform scaling or columnwisebandmatrices of scalars.This also makes the
get_index
for field matrices return a field instead of a broadcasted object when the keys used to index the field matrix will result in a columnwisebandmatrices of scalars.get_internal_entry
, which is called byget_index
for FieldMatrices, is not complete, but works for everything thatscalar_fieldmatrix
needs. Any indexing that was previously supported is still supported. An example of something that could be added, is if an entry is a tuple of axistensors, indexing into only the axis tensor components.